Summary of Changes

Hello @devs6186, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of capa's .NET file analysis by gracefully handling corrupted or malformed .NET metadata. Previously, such files could lead to unhandled struct.error exceptions, causing the analysis to fail abruptly without a clear explanation. The changes introduce specific error trapping and re-raise a more informative exception, providing users with better feedback when encountering problematic .NET executables.

Highlights

• Robust Error Handling: Implemented error handling for struct.error exceptions that can occur when dnfile.dnPE() attempts to parse malformed or truncated .NET metadata.
• Clearer User Feedback: When a struct.error is caught, it is now re-raised as a capa.loader.CorruptFile exception with a user-friendly message: "Invalid or truncated .NET metadata; the file may be corrupted or not a valid .NET PE."
• Dependency Import: The struct module was imported in capa/features/extractors/dnfile/extractor.py and capa/features/extractors/dotnetfile.py to support the new error handling.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Thanks for the review! Glad the approach looks right.

One small thing worth calling out on the DnfileFeatureExtractor side: super().__init__() is intentionally called after the dnPE() call rather than before, because computing SampleHashes.from_bytes() in the parent is cheap, but if parsing blows up on a malformed file it's cleaner to fail fast before we've done any work. The ordering in DotnetFileFeatureExtractor already had super().__init__() first (since it reads hashes independently), so I kept that as-is.

No other changes needed based on the feedback — both extractors are now consistent in how they surface this error to the caller.

Would a helper function make sense to only write the except handling code once?
I'd also like to see the provided exception details vs. assuming it's a specific parsing error. What do you think?

Would a helper function make sense to only write the except handling code once? I'd also like to see the provided exception details vs. assuming it's a specific parsing error. What do you think?

Well both suggestions make sense and I can incorporate them.

1. Helper function
   Agreed. Duplicating the try/except struct.error in extractor.py and dotnetfile.py isn’t ideal. I’ll add a small helper (e.g. in loader or in a shared dnfile helper used by both) that performs the dnPE() call and centralizes the handling. Both call sites will then use that helper so the except logic lives in one place.
2. Exception details
   Also agreed. Using a fixed message (“Invalid or truncated .NET metadata…”) hides the real cause. I’ll change it so we surface the underlying error, e.g. by including str(e) in the CorruptFile message, e.g.
   "Invalid or truncated .NET metadata: {original_message}"
   (or similar wording). That way we don’t assume it’s “a specific parsing error” and users get the actual reason.
   Just let me know, ill update these changes and push a commit!

@devs6186 CI tests are failing:

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /home/runner/work/capa/capa/capa/features/extractors/dotnetfile.py
Skipped 4 files

All changes made by hooks:
diff --git a/capa/features/extractors/dotnetfile.py b/capa/features/extractors/dotnetfile.py
index 3c3d83b..919f4ff 100644
--- a/capa/features/extractors/dotnetfile.py
+++ b/capa/features/extractors/dotnetfile.py
@@ -12,8 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import logging
 import struct
+import logging
 from typing import Iterator
 from pathlib import Path

Please ensure all unit tests are passing locally before requesting another review, thank you!

@devs6186 CI tests are failing:

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /home/runner/work/capa/capa/capa/features/extractors/dotnetfile.py
Skipped 4 files

All changes made by hooks:
diff --git a/capa/features/extractors/dotnetfile.py b/capa/features/extractors/dotnetfile.py
index 3c3d83b..919f4ff 100644
--- a/capa/features/extractors/dotnetfile.py
+++ b/capa/features/extractors/dotnetfile.py
@@ -12,8 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import logging
 import struct
+import logging
 from typing import Iterator
 from pathlib import Path

Please ensure all unit tests are passing locally before requesting another review, thank you!

yes @mike-hunhoff i just noticed, ill take a days time not more than that and look into all the comments and changes requested for this pr and all the others and get back to you asap, thank you for your patience !

Would a helper function make sense to only write the except handling code once? I'd also like to see the provided exception details vs. assuming it's a specific parsing error. What do you think?

Well both suggestions make sense and I can incorporate them.

1. Helper function
   Agreed. Duplicating the try/except struct.error in extractor.py and dotnetfile.py isn’t ideal. I’ll add a small helper (e.g. in loader or in a shared dnfile helper used by both) that performs the dnPE() call and centralizes the handling. Both call sites will then use that helper so the except logic lives in one place.
2. Exception details
   Also agreed. Using a fixed message (“Invalid or truncated .NET metadata…”) hides the real cause. I’ll change it so we surface the underlying error, e.g. by including str(e) in the CorruptFile message, e.g.
   "Invalid or truncated .NET metadata: {original_message}"
   (or similar wording). That way we don’t assume it’s “a specific parsing error” and users get the actual reason.
   Just let me know, ill update these changes and push a commit!

@devs6186 we'll give this another review once you've addressed these changes, thank you!

Would a helper function make sense to only write the except handling code once? I'd also like to see the provided exception details vs. assuming it's a specific parsing error. What do you think?

Well both suggestions make sense and I can incorporate them.

1. Helper function
   Agreed. Duplicating the try/except struct.error in extractor.py and dotnetfile.py isn’t ideal. I’ll add a small helper (e.g. in loader or in a shared dnfile helper used by both) that performs the dnPE() call and centralizes the handling. Both call sites will then use that helper so the except logic lives in one place.
2. Exception details
   Also agreed. Using a fixed message (“Invalid or truncated .NET metadata…”) hides the real cause. I’ll change it so we surface the underlying error, e.g. by including str(e) in the CorruptFile message, e.g.
   "Invalid or truncated .NET metadata: {original_message}"
   (or similar wording). That way we don’t assume it’s “a specific parsing error” and users get the actual reason.
   Just let me know, ill update these changes and push a commit!

@devs6186 we'll give this another review once you've addressed these changes, thank you!

i have addressed them now in the latest commit

@devs6186 lints are failing. Please ensure all lints pass locally before requesting another review.